Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenBSD tcpmd5 #5

Draft
wants to merge 14 commits into
base: dev
Choose a base branch
from
Draft

OpenBSD tcpmd5 #5

wants to merge 14 commits into from

Conversation

robertkeizer
Copy link

Context / Background

I wanted to use bgpipe on OpenBSD with the TCP MD5 signatures option. I was able to fairly easily "make it work", but I'd like to see about supporting the platform more generally.

On OpenBSD to facilitate TCP MD5 signatures, a security association must be setup on the system, in addition to setting a flag on the socket. I also found that unix.TCP_MD5SIG_EXT was not available.

I was able to use unix.SetsockoptString(int(fd), unix.IPPROTO_TCP, 0x04, string("...")) in addition to previously loading a security association on the machine to make this work.

To load the security association I used ipsecctl -f /tmp/tcpmd5.conf with the following defined ( roughly )

/tmp/tcpmd5.conf

tcpmd5 from 198.51.100.1 to 198.51.100.2 spi 0x100 authkey 0x0000000000
tcpmd5 from 198.51.100.2to 198.51.100.1 spi 0x101 authkey 0x0000000000

On OpenBSD I would expect most programs to not define and create the security associations, but rather error out if the security associations weren't defined and tcp md5 signatures were required.

Question

  • What are your thoughts on supporting OpenBSD?
  • Would you prefer to:
    • Attempt to dynamically create the security associations in favour of usability?
    • Error out if the md5 flag is set and a security association could not found or there is an error setting the socket option?

I would strongly suspect most people running OpenBSD would prefer the latter (simply error out), but this isn't my project, so I figure opening a draft PR and asking the question is the more appropriate path.

@pforemski
Copy link
Contributor

Hey Robert! So nice to see such a great PR, and I'm 100% for supporting OpenBSD, and the *BSD platforms in general.

However, it's been a while since I last run and administered an OpenBSD box (oh man, the 5.0 release was so long ago) and I need some time to get up to speed in order to respond to your PR, OK?

Some context for this work I can think of:

  • this project needs to prioritize automated testing, and this PR should add OpenBSD (and maybe OpenBGPD too) into the scope for testing
  • my dev pipeline currently processes ADD_PATH support, plus some more features, in the dev branch (see ADD_PATH and other features bgpfix#5) - which should be merged into the main branch soon - when this happens, do you think you could re-base your commits on top of that? in the meantime, I'll try to go through your changes and make my mind as to how to support OpenBSD in bgpipe

@pforemski
Copy link
Contributor

@robertkeizer A few comments as I'm back to this PR:

  1. I've released bgpipe v0.9.0 and merged ADD_PATH, grep, and caps #6 into main. Could you please re-base your work on the top of the new dev branch?

  2. I'm trying to understand the OpenBSD's TCP-MD5 context and expected behavior. IIRC, you think people would expect bgpipe to fail to start if TCP-MD5 was requested but security association was not setup. However, I found that OpenBGPD will happily setup the keys if configured in bgpd.conf(5): see https://github.com/openbgpd-portable/openbgpd-openbsd/blob/master/src/usr.sbin/bgpd/pfkey.c#L532. Thus, I'm a bit towards the OpenBGPD way (setup the association if needed), but also provide a flag (eg. --ipsec) to skip that and do things the way you suggested. What do you think?

  3. Is there anything I could do to help you in finishing this PR? I really like your initiative and would love to see this merged and released soon. Thank you!

@robertkeizer
Copy link
Author

Hey @pforemski , thanks for the feedback.

  1. Absolutely; I'll rebase and also clean it up a bit in terms of commits.

  2. After thinking about this a bit more, I think you're 100% right; Ideally the TCP-MD5 security association should be attempted to be setup and "just work" if possible. My original thinking was flawed, and if a permission error occurs around not running bgpipe with the permissions to create the security association, erroring out at that point makes sense.

I'm wondering what happens to the security assocation if bgpipe sets one up and is subsequently killed or exits abnormally, but I suppose that's an easy RTFM and test as part of the PR work really.

  1. You've done that by responding to the PR and being clear about what you think should be done and why. This PR was very much a "I made it work, I should at least clean it up a bit and toss a draft PR in to see what next steps should be" kind of situation

@robertkeizer robertkeizer changed the base branch from main to dev October 11, 2024 18:38
@pforemski
Copy link
Contributor

Great, let me know if/when I can be of any help here. Happy to test this when you're done on OpenBSD 7.6 :-) (take your time, no rush)

@robertkeizer
Copy link
Author

Using sr.ht for some OpenBSD builds and tests ( this link for reference ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@robertkeizer @pforemski